feat(iso): surface structured error fields on the state-aware wire envelope by adpa-ms · Pull Request #708 · microsoft/mxc

adpa-ms · 2026-07-29T20:55:11Z

📖 Description

IsolationSession failures arrive as one concatenated string in error.message — operation, HRESULT, API message and remediation hint flattened together — so callers have to parse prose. This promotes them to discrete fields on the state-aware error envelope: operation, nativeCode and remediation. message becomes the bare text, which for a semantic API failure is the API's own message passed through verbatim.

The three fields live in a boxed ApiFailure on MxcError. Grouping them makes the invariant the easy path — nativeCode and remediation sit beside operation rather than as independent optionals, so the normal construction path cannot produce a status without the call it came from — and keeps MxcError under clippy's result_large_err threshold. Classification is extracted into pure functions so the rules are unit-testable; IsoSessionError is WinRT-activated and cannot be constructed in a test.

The TypeScript MxcError gains a constructor overload taking a flat object mirroring the wire shape; the positional signature is retained and declared last, so existing callers and ConstructorParameters are unaffected.

Out of scope by design: the one-shot path keeps composing its full string, having no structured envelope to read fields from. Windows Sandbox has no semantic error channel analogous to IsoSessionError, so the fields would be uniformly absent — the reference doc now says so explicitly rather than implying the fields are universal. A follow-on can unify the one-shot envelope and carry the fields to the C# SDK.

⚠️ Observable changes

The envelope shape is backward-compatible — every new field is optional and absent unless an API call was in flight. Two content changes are worth checking before you merge:

message no longer embeds the operation and HRESULT for a failure raised by the IsolationSession API; they moved to their own fields. Anything that greps the message string for an HRESULT needs to read nativeCode instead. MxcError's Display re-attaches them (code: message [operation nativeCode]) so log-only consumers keep the detail, but the wire message is bare.
Provision-time ERROR_NOT_FOUND now returns backend_error, not stale_id. The promotion applied to every operation, including provision, which cannot produce a stale id because it mints the sandbox — both the doc comment and docs/isolation-session/state-aware-rust.md already specified non-provision-only, so this aligns the code with its stated contract. Retry logic branching on stale_id during provision behaves differently. The promotion stays semantic-path only by design: a transport HRESULT of the same value lacks the provenance that gives it that meaning, so promoting it would emit a false stale_id and tell the caller to destroy a healthy sandbox.

🔍 Validation

Tests were added for every decision above, so a later rebase that silently undoes one fails loudly — field population, the invariant across all variants, the camelCase serde rename, both halves of the stale_id rule, the legacy positional constructor, and the full chain through the SDK.

Windows x64: fmt, clippy -D warnings, build and test with isolation_session ON and OFF, wxc_host_prep elevated, the versioning and .NET parity gates, SDK build/unit/integration. Isolation-capable host (OS build 26662.1004): one-shot, state-aware and SDK Node E2E suites all green (73 passed, 0 failed), plus the manual interactive TTY tests. No agent users leaked, verified as a diff against a pre-run snapshot of the full local-account set.

The four host-independent policy-validation cases moved out of the runtime-probe-gated suite. They are rejections MXC raises before any IsolationSession API call, so they need wxc-exec built with the feature but not a host that can run isolation sessions — they now execute in normal CI, where the whole suite previously skipped.

Observed from the live API on the stale_id path:

{"error":{"code":"stale_id","message":"The provision was not found.","operation":"IsoSessionOps.StopSessionAsync","nativeCode":"0x80070490"}}

✅ Checklist

Signed the Contributor License Agreement
Updated documentation (if applicable)

📋 Issue Type

Feature

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-07-29T20:55:20Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

…ability Addresses both optional findings from the review of #708. O1 -- the wire `message` could be empty. `Message()` is a best-effort WinRT getter, and with the operation and HRESULT now in their own fields nothing backfills `message`, so a failed or empty getter reached the wire as `"message": ""`. The change was internally inconsistent about it: the `Err(Code())` arm already guarded the empty case, and `remediation` normalised empty-to-absent, but the `Ok(code)` arm passed the raw string through. Both best-effort getters now collapse to `Option` at the WinRT boundary and `IsoApiFailure::new` decides what absent means per field -- a stand-in for `message`, which the wire requires, and absence for `remediation`, which is optional. Normalising at construction rather than per branch is what keeps the guarantee from having to be restated at each call site; every construction path routes through it. O2 -- `operation` values are now published in the SDK README, recommended for telemetry aggregation, and pinned by an E2E assertion, but nothing said whether they are stable. They mirror the projected WinRT class and method names, which this repo does not own and cannot version, so they are now documented as best-effort diagnostics rather than a versioned contract, in the cross-backend contract, the backend spec, and the SDK README. The E2E assertion that pins an exact value carries a note explaining why pinning is correct there specifically: it verifies MXC's own mapping and moves with the constant. Also verified the boxing rationale the review could not check without running clippy: `MxcError` is 72 bytes as written and would be 136 inlined, against the default 128-byte `result_large_err` threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2

Copilot

Pull request overview

Adds structured platform API failure details to state-aware error envelopes and the Node SDK.

Changes:

Adds operation, nativeCode, and remediation fields.
Refactors IsolationSession error classification and stale-ID handling.
Adds Rust, SDK, integration, and live API coverage.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`tests/scripts/run_isolation_session_state_aware_tests.ps1`	Tests live structured errors.
`src/core/wxc_common/src/mxc_error.rs`	Extends error envelopes.
`src/backends/isolation_session/common/src/process_options.rs`	Structures option errors.
`src/backends/isolation_session/common/src/manager.rs`	Propagates structured failures.
`src/backends/isolation_session/common/src/error.rs`	Implements error classification.
`sdk/node/tests/unit/state-aware.test.ts`	Tests envelope parsing.
`sdk/node/tests/unit/errors.test.ts`	Tests SDK error APIs.
`sdk/node/tests/integration/isolation-session-state-aware.test.ts`	Tests SDK failure propagation.
`sdk/node/src/state-aware.ts`	Preserves exec error fields.
`sdk/node/src/state-aware-helper.ts`	Parses complete envelopes.
`sdk/node/src/sandbox.ts`	Preserves one-shot envelope fields.
`sdk/node/src/index.ts`	Exports new error types.
`sdk/node/src/errors.ts`	Extends `MxcError`.
`sdk/node/README.md`	Documents error handling.
`docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md`	Defines the wire contract.
`docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md`	Updates the design overview.
`docs/isolation-session/state-aware-rust.md`	Documents backend behavior.

Gudge (MGudgin)

I think this is pretty much as we discussed. Some comments from findings by my review skill.

Gudge (MGudgin) · 2026-07-31T16:47:38Z

+export interface MxcErrorFields {
+  /** Machine-readable category. Branch on this first. */
+  code: ErrorCode;
+  /** Human-readable description of the failure. */
+  message: string;
+  /**
+   * The underlying API call that failed, namespaced by its interface — e.g.
+   * `IsoSessionOps.RunProcessWithOptionsAsync`. Low-cardinality and free of
+   * call parameters, so it is safe to group on in telemetry.
+   */
+  operation?: string;
+  /**
+   * The underlying platform status as a string — an HRESULT such as
+   * `0x80070490` on Windows, an errno or equivalent elsewhere.
+   */
+  nativeCode?: string;
+  /** The API's actionable "how to fix it" hint, when it supplied one. */
+  remediation?: string;
+  /**
+   * Open extension point for backend-specific structured data that has no
+   * dedicated field. Named fields are reserved for backend-neutral concepts.
+   */
+  details?: Record<string, unknown>;
+}
+
+/**
+ * The `error` arm of a wire response envelope, as received from the
+ * executor. Identical to {@link MxcErrorFields} except that `code` is an
+ * open `string`: an unrecognised code is passed through verbatim rather than
+ * being coerced or dropped.
+ */
+export interface WireError extends Omit<MxcErrorFields, 'code'> {
+  code: string;
+}


[Medium · proportionality] Internal envelope parsing became permanent public API

The feature needs the new readonly properties on MxcError. It does not obviously need
MxcErrorFields and WireError exported, nor the object-form constructor overload, nor
mxcErrorFromEnvelope — these serve three internal parsing sites, but they are
re-exported from sdk/node/src/index.ts, which makes them supported surface the SDK has
to keep working indefinitely.

Public API is the hardest thing to walk back: once these ship, a consumer can build
against WireError and any later change to the wire-parsing internals becomes a
breaking change to them, for no benefit either side asked for.

Suggested: keep MxcErrorFields, WireError and mxcErrorFromEnvelope module-internal
(drop them from index.ts), preserve the existing public constructor signature, and
export only the new readonly operation / nativeCode / remediation properties. If
the object-form constructor turns out to be wanted externally, adding it later is
additive and safe — removing it is not.

Accepted for two of the three, with a specific reason for keeping the third.

Your premise checks out exactly: the base branch exported ErrorCode, MxcError, mxcErrorFromCode, and this PR added exactly three more for three internal call sites. WireError and mxcErrorFromEnvelope are now un-exported from index.ts in d453262. All three consumers import from ./errors.js directly rather than through the barrel, so it was zero-churn. The SDK's job is to parse envelopes, so a consumer shouldn't need the helper.

MxcErrorFields I've kept exported, because un-exporting it doesn't reduce the public surface — it's the parameter type of a public constructor overload on a public class. I built a standalone reproduction to check rather than reason about it. With the interface un-exported and declaration: true, it compiles (exit 0) and emits:

interface MxcErrorFields { ... } // declared, not exported export declare class MxcError extends Error { constructor(fields: MxcErrorFields); // public ctor still references it } export {};

A consumer can then still call new MxcError({ code, message, operation }) — structural typing — but import type { MxcErrorFields } fails with TS2459: Module declares 'MxcErrorFields' locally, but it is not exported. So we'd carry the identical commitment while making it impossible to name in a helper signature or variable annotation.

Reproduction details so this is checkable: TypeScript 5.9.3, "declaration": true, "strict": true, "module"/"moduleResolution": "NodeNext" — i.e. the SDK's own tsconfig.json settings. The consumer-side failure is on import type { MxcErrorFields }; the new MxcError({ ... }) call on the line above it compiles fine, which is precisely the asymmetry that makes un-exporting counterproductive.

Also relevant to the overload's value: errors.test.ts has keeps ConstructorParameters resolving to the positional form, pinning ConstructorParameters<typeof MxcError> to ['stale_id', 'boom']. The overload ordering is load-bearing and already guarded, so the object form is additive at the type level too.

The only way to genuinely remove MxcErrorFields from the public surface is to drop the object-form overload, which is a larger change than the one proposed and would give up the non-breaking migration path the overload exists to provide. If you think that's the right trade I'm open to discussing it separately, but it seemed out of proportion to the concern as written.

Gudge (MGudgin) · 2026-07-31T16:47:38Z

+/// Whether an `ERROR_NOT_FOUND` from this operation means "the sandbox is
+/// gone".
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(super) enum StalePromotion {
+    /// Non-provision operations address an existing agent user, so
+    /// `ERROR_NOT_FOUND` means that user is gone.
+    Eligible,
+    /// Provision *mints* the agent user. There is no `sandboxId` yet, so
+    /// reporting `stale_id` — whose remediation is "re-provision; treat the
+    /// id as dead" — would be incoherent.
+    NotEligible,
+}


[Medium · proportionality] + [Low · backward-compatibility] The stale_id narrowing is a separate, wire-visible change

Two axes landed on this same code, from different directions — recording both here.

Proportionality: StalePromotion changes provision-time ERROR_NOT_FOUND from
stale_id to backend_error. That is an independently observable classification
change, not plumbing required by the structured-fields feature. It rides along in a PR
whose stated purpose is promoting error components to discrete fields, which means the
two cannot be reviewed, reverted, or bisected independently. Suggest splitting it (and
its tests) into its own PR — it is a clean, self-contained fix that would land fast on
its own merits.

Backward-compatibility: it is a wire-visible change to the emitted code for an
existing failure path. The old behaviour was incoherent — the doc comment here makes
that case well, and I agree with the direction — but any caller with retry logic
branching on stale_id during provision behaves differently after this merges. That
deserves an explicit line in the PR description / release notes, not just a code
comment, so downstream consumers can check their handling.

No change requested to the logic itself; the Eligible / NotEligible split and the
"semantic-path only" reasoning in the doc comment below are both right, and the
"Do not 'fix' the asymmetry" note is a good thing to have written down.

Taking these in reverse order, because I'm accepting one and pushing back on the other.

On the release-note point — accepted. You're right that a code comment isn't where a consumer looks. The PR description now has an Observable changes section stating plainly that provision-time ERROR_NOT_FOUND returns backend_error instead of stale_id, and that retry logic branching on stale_id during provision behaves differently.

On splitting it into its own PR — I'd like to push back, on the grounds that it isn't actually an independent behavior change. Both descriptions of that code on the base branch already specified the narrow behavior:

the base code comment: "Every non-provision lifecycle op (start / exec / stop / deprovision) surfaces this HRESULT… we promote it to Stale"

docs/isolation-session/state-aware-rust.md:227 on the base branch: "After deprovision, every non-provision op against the dead sandboxId triggers this."

Only the code disagreed — it promoted unconditionally. So this aligns the code with a contract that was already written down in two places, and the rewrite is what forced the discrepancy into the open.

Both quotations are verifiable at the merge base, 86ceab7f5acb014e4af0b55fdfaa820718476bfb ("fix(isolation-session): refuse network policy the backend cannot enforce… (#682)"):

git show 86ceab7:src/backends/isolation_session/common/src/error.rs git show 86ceab7:docs/isolation-session/state-aware-rust.md

In the first, the promotion is if code == ERROR_NOT_FOUND_HRESULT { IsolationSessionError::Stale(formatted) } — no operation filter — sitting directly beneath the comment claiming non-provision-only. The doc quote is line 227 of the second.

I also searched every stale_id reference outside Rust: stale_id|StaleId across sdk/, tests/ and docs/, 43 matches. The only behavioral consumers are tests/scripts/run_isolation_session_state_aware_tests.ps1:634 (stop on a deprovisioned sandbox), docs/isolation-session/state-aware-rust.md:192 (the deprovision row) and sdk/node/README.md:277 (generic re-provision guidance) — all non-provision. Nothing in-tree branches on provision-time stale_id, and no doc ever described it as reachable.

Splitting is also awkward in both directions. Backward means reimplementing the narrowing against the pre-rewrite format_iso_error that this PR deletes, then rebasing through it. Forward means shipping Eligible everywhere and leaving the code contradicting two docs that are themselves in this PR.

Given you've endorsed the logic and the behavior is now called out explicitly, I'd rather keep it here than pay that churn. Happy to be overruled if you still think the bisect story justifies it.

…velope Promote the components of an IsolationSession failure out of the concatenated `message` string and into discrete fields on the wire error envelope: `operation`, `nativeCode` and `remediation`, alongside the existing `code` and `message`. On the state-aware path `message` becomes the bare human-readable text; for a semantic API failure that is the API's own message, passed through verbatim. Wire model (`wxc_common::mxc_error`) - `ApiFailure { operation, native_code?, remediation? }`, held boxed on `MxcError`. Grouping makes the envelope invariant unrepresentable to violate -- `nativeCode` and `remediation` cannot exist without `operation` -- and keeps `MxcError` small enough that every `Result<_, MxcError>` in the workspace stays under clippy's `result_large_err` threshold. - `ErrorEnvelope` gains the three fields, each omitted when unset; `native_code` serialises as camelCase `nativeCode`. IsolationSession backend - `Lifecycle`/`Stale` carry the components structurally instead of a pre-formatted string; `LifecycleFailure::Internal` makes an MXC-side failure structurally incapable of naming an API operation. - Classification split into a pure function so the rules are unit-testable -- `IsoSessionError` is WinRT-activated and cannot be constructed in a test. Same split applied to the activation-failure mapping, which now reports `backend_unavailable` with its operation and HRESULT. - `operation` is interface-qualified, low-cardinality and parameter-free (a failing environment insert names the variable in `message`). - Fixes a latent bug: the `ERROR_NOT_FOUND` -> `stale_id` promotion applied to provision too, which cannot produce a stale id because it mints the sandbox. It is now restricted to non-provision operations, and stays semantic-path only -- a transport HRESULT of the same value has none of the provenance that gives it that meaning, so promoting it would emit a false `stale_id` and tell the caller to destroy a healthy sandbox. One-shot is deliberately untouched: `Display` still composes the full human string, including the category prefix, because that path has no structured envelope to read the fields from. TypeScript SDK - `MxcError` gains a constructor overload taking a flat `MxcErrorFields` object mirroring the wire shape. The positional signature is retained and declared last, so existing callers and `ConstructorParameters<typeof MxcError>` are unaffected. - `mxcErrorFromEnvelope` is the single wire-to-error boundary, including the unknown-code passthrough; all envelope-parsing sites route through it. Also removes several pre-existing OS-internal names from prose in the files touched, per repo convention. Verified on the retail host: cargo fmt, clippy (--all-features), build and test with isolation_session ON and OFF, SDK unit, SDK integration, the versioning gate suite, and wxc_host_prep in an elevated shell -- all green. The iso E2E suites still need a VM run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2

…ability Addresses both optional findings from the review of #708. O1 -- the wire `message` could be empty. `Message()` is a best-effort WinRT getter, and with the operation and HRESULT now in their own fields nothing backfills `message`, so a failed or empty getter reached the wire as `"message": ""`. The change was internally inconsistent about it: the `Err(Code())` arm already guarded the empty case, and `remediation` normalised empty-to-absent, but the `Ok(code)` arm passed the raw string through. Both best-effort getters now collapse to `Option` at the WinRT boundary and `IsoApiFailure::new` decides what absent means per field -- a stand-in for `message`, which the wire requires, and absence for `remediation`, which is optional. Normalising at construction rather than per branch is what keeps the guarantee from having to be restated at each call site; every construction path routes through it. O2 -- `operation` values are now published in the SDK README, recommended for telemetry aggregation, and pinned by an E2E assertion, but nothing said whether they are stable. They mirror the projected WinRT class and method names, which this repo does not own and cannot version, so they are now documented as best-effort diagnostics rather than a versioned contract, in the cross-backend contract, the backend spec, and the SDK README. The E2E assertion that pins an exact value carries a note explaining why pinning is correct there specifically: it verifies MXC's own mapping and moves with the constant. Also verified the boxing rationale the review could not check without running clippy: `MxcError` is 72 bytes as written and would be 136 inlined, against the default 128-byte `result_large_err` threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2

Fixes found by review of the structured-error-fields change. - transport_err no longer emits a dangling "step: " when the platform supplies no message text. An HRESULT with no OS message-table entry (0xDEADBEEF, and any custom facility code) returns an empty message(), and joining unconditionally produced a technically-non-empty string that slipped past the empty-message guard in IsoApiFailure::new. Fall back to the step alone. ~34 call sites route through this one join. - MxcError::Display now renders the API detail when present, so a consumer that only logs the error keeps the operation and status that used to be concatenated into message. Rendering only: the wire envelope still carries message bare, with the components in their own fields. Replaces the thiserror derive with explicit Display + Error. - The Code()-getter-failure branch moves into unreadable_code_failure, a pure function, so its composition is reachable from a unit test. format_iso_error stays a thin WinRT adapter. - Correct the ApiFailure doc comment: grouping makes the invariant the easy path, not an unrepresentable-to-violate one (Default was derived and the fields are pub). Drop the unused Default derive. - Add #[non_exhaustive] to MxcError and ErrorEnvelope so future fields are a non-event for other workspace crates. - Guard the MxcError constructor against a nullish argument, which took the object branch and failed inside super() with a TypeError naming "message". Default the positional message rather than asserting it. - Un-export WireError and mxcErrorFromEnvelope: they exist so the SDK's own parse sites share one widening point. MxcErrorFields stays exported because it is the parameter type of a public constructor overload -- hiding the name leaves the type usable but unnameable. - Lift the four host-independent policy-validation cases out of the probe-gated suite. Both CI systems set MXC_SKIP_OS_BUILD_DEPENDENT_TESTS=1, so nothing in that suite ran in CI; these need the isolation_session feature compiled in but not a host that can run isolation sessions. - Document that the structured fields are currently populated only by IsolationSession state-aware operations. Gates: fmt; clippy --all-targets --all-features -D warnings; Rust build + test iso ON and iso OFF (wxc_host_prep 16/16 elevated); SDK unit 231/0; SDK integration 45/0 with the four lifted cases now executing under the CI skip flag; versioning + dotnet parity 7/7; VM suites 73 passed / 0 failed with an empty leak delta; manual TTY tests confirmed by the operator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2

…velope (#708) * feat(iso): surface structured error fields on the state-aware wire envelope Promote the components of an IsolationSession failure out of the concatenated `message` string and into discrete fields on the wire error envelope: `operation`, `nativeCode` and `remediation`, alongside the existing `code` and `message`. On the state-aware path `message` becomes the bare human-readable text; for a semantic API failure that is the API's own message, passed through verbatim. Wire model (`wxc_common::mxc_error`) - `ApiFailure { operation, native_code?, remediation? }`, held boxed on `MxcError`. Grouping makes the envelope invariant unrepresentable to violate -- `nativeCode` and `remediation` cannot exist without `operation` -- and keeps `MxcError` small enough that every `Result<_, MxcError>` in the workspace stays under clippy's `result_large_err` threshold. - `ErrorEnvelope` gains the three fields, each omitted when unset; `native_code` serialises as camelCase `nativeCode`. IsolationSession backend - `Lifecycle`/`Stale` carry the components structurally instead of a pre-formatted string; `LifecycleFailure::Internal` makes an MXC-side failure structurally incapable of naming an API operation. - Classification split into a pure function so the rules are unit-testable -- `IsoSessionError` is WinRT-activated and cannot be constructed in a test. Same split applied to the activation-failure mapping, which now reports `backend_unavailable` with its operation and HRESULT. - `operation` is interface-qualified, low-cardinality and parameter-free (a failing environment insert names the variable in `message`). - Fixes a latent bug: the `ERROR_NOT_FOUND` -> `stale_id` promotion applied to provision too, which cannot produce a stale id because it mints the sandbox. It is now restricted to non-provision operations, and stays semantic-path only -- a transport HRESULT of the same value has none of the provenance that gives it that meaning, so promoting it would emit a false `stale_id` and tell the caller to destroy a healthy sandbox. One-shot is deliberately untouched: `Display` still composes the full human string, including the category prefix, because that path has no structured envelope to read the fields from. TypeScript SDK - `MxcError` gains a constructor overload taking a flat `MxcErrorFields` object mirroring the wire shape. The positional signature is retained and declared last, so existing callers and `ConstructorParameters<typeof MxcError>` are unaffected. - `mxcErrorFromEnvelope` is the single wire-to-error boundary, including the unknown-code passthrough; all envelope-parsing sites route through it. Also removes several pre-existing OS-internal names from prose in the files touched, per repo convention. Verified on the retail host: cargo fmt, clippy (--all-features), build and test with isolation_session ON and OFF, SDK unit, SDK integration, the versioning gate suite, and wxc_host_prep in an elevated shell -- all green. The iso E2E suites still need a VM run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2 * fix(iso): never emit an empty error message; state operation-value stability Addresses both optional findings from the review of #708. O1 -- the wire `message` could be empty. `Message()` is a best-effort WinRT getter, and with the operation and HRESULT now in their own fields nothing backfills `message`, so a failed or empty getter reached the wire as `"message": ""`. The change was internally inconsistent about it: the `Err(Code())` arm already guarded the empty case, and `remediation` normalised empty-to-absent, but the `Ok(code)` arm passed the raw string through. Both best-effort getters now collapse to `Option` at the WinRT boundary and `IsoApiFailure::new` decides what absent means per field -- a stand-in for `message`, which the wire requires, and absence for `remediation`, which is optional. Normalising at construction rather than per branch is what keeps the guarantee from having to be restated at each call site; every construction path routes through it. O2 -- `operation` values are now published in the SDK README, recommended for telemetry aggregation, and pinned by an E2E assertion, but nothing said whether they are stable. They mirror the projected WinRT class and method names, which this repo does not own and cannot version, so they are now documented as best-effort diagnostics rather than a versioned contract, in the cross-backend contract, the backend spec, and the SDK README. The E2E assertion that pins an exact value carries a note explaining why pinning is correct there specifically: it verifies MXC's own mapping and moves with the constant. Also verified the boxing rationale the review could not check without running clippy: `MxcError` is 72 bytes as written and would be 136 inlined, against the default 128-byte `result_large_err` threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2 * fix(errors): address PR review round 2 Fixes found by review of the structured-error-fields change. - transport_err no longer emits a dangling "step: " when the platform supplies no message text. An HRESULT with no OS message-table entry (0xDEADBEEF, and any custom facility code) returns an empty message(), and joining unconditionally produced a technically-non-empty string that slipped past the empty-message guard in IsoApiFailure::new. Fall back to the step alone. ~34 call sites route through this one join. - MxcError::Display now renders the API detail when present, so a consumer that only logs the error keeps the operation and status that used to be concatenated into message. Rendering only: the wire envelope still carries message bare, with the components in their own fields. Replaces the thiserror derive with explicit Display + Error. - The Code()-getter-failure branch moves into unreadable_code_failure, a pure function, so its composition is reachable from a unit test. format_iso_error stays a thin WinRT adapter. - Correct the ApiFailure doc comment: grouping makes the invariant the easy path, not an unrepresentable-to-violate one (Default was derived and the fields are pub). Drop the unused Default derive. - Add #[non_exhaustive] to MxcError and ErrorEnvelope so future fields are a non-event for other workspace crates. - Guard the MxcError constructor against a nullish argument, which took the object branch and failed inside super() with a TypeError naming "message". Default the positional message rather than asserting it. - Un-export WireError and mxcErrorFromEnvelope: they exist so the SDK's own parse sites share one widening point. MxcErrorFields stays exported because it is the parameter type of a public constructor overload -- hiding the name leaves the type usable but unnameable. - Lift the four host-independent policy-validation cases out of the probe-gated suite. Both CI systems set MXC_SKIP_OS_BUILD_DEPENDENT_TESTS=1, so nothing in that suite ran in CI; these need the isolation_session feature compiled in but not a host that can run isolation sessions. - Document that the structured fields are currently populated only by IsolationSession state-aware operations. Gates: fmt; clippy --all-targets --all-features -D warnings; Rust build + test iso ON and iso OFF (wxc_host_prep 16/16 elevated); SDK unit 231/0; SDK integration 45/0 with the four lifted cases now executing under the CI skip flag; versioning + dotnet parity 7/7; VM suites 73 passed / 0 failed with an empty leak delta; manual TTY tests confirmed by the operator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2 --------- Co-authored-by: adpa-ms <> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67a135de-4549-4c4e-8428-6f2a993e9bf2

Enterprise support is not ready to ship in main. This removes the Entra `user` API (`{ upn, wamToken }`) from the isolation_session backend so the main-bound branch carries no enterprise surface. It is restored on feature/isolation-session-internal by the commit that follows. Scope is strictly the `user` API. Everything else on the branch -- the IsolationSession Preview API migration, `appId` in the structured `sandboxId`, structured error fields, and the network/UI policy rejection work -- is untouched. Rust - Wire model: drop `IsolationUser`, `IsolationSessionStartPhase`, the `start` slot on `IsolationSession`, and `user` from the provision phase. Provision is now the only phase carrying a per-phase wire object, which is what the generated schema advertises. - Domain model: drop `IsolationSessionUser` and `IsolationSessionStartConfig`. The latter's only field was `user`, so the type has nothing left to carry -- matching Rust's existing pattern, where a phase type exists only if the phase contributes a wire object (exec/stop/deprovision already use `()`). #683 made exactly this change for stop and deprovision; start now joins them. - Backend: `type StartConfig = ()`. Remove `os_credentials` and the start-phase shape validation. `IsoSessionOps.AddUserAsync` / `StartSessionAsync` keep the OS-defined optional account/token parameters -- MXC now always passes empty strings, which is what the local-agent path already did -- so the generated `bindings.rs` is untouched. - `policy.rs`: remove `validate_isolation_session_user`. TypeScript - Remove the `IsolationSessionUserConfig` class (and its `wamToken` inspect redaction) and the `user` fields on the provision and start configs. - KEEP `IsolationSessionStartConfig` as `{ version?: string }`. Deleting it would break the pattern rather than follow it: five sibling interfaces are already version-only, including `WindowsSandboxStartConfig` -- the same phase on the other state-aware backend -- and `ConfigsForBackend` requires all five phase keys per backend. `version` is also not vestigial: state-aware-helper lifts it out of the backend object onto the envelope as the request's schema version, so removing the type would leave `start` on isolation_session as the only (backend, phase) pair a caller cannot version. Tests - Wire-conformance: the start-phase equivalence assertions are replaced by `_StartNoBackendKeys`, joining the existing exec/stop/deprovision group. Deleting only the failing `_StartKeysNonVacuous` guard and keeping the equivalences would have left three assertions passing because both sides are `never` -- vacuously true, which is precisely what that guard exists to catch. - `phases_without_a_config_reject_a_payload` now covers `start`, pinning that it moved into the no-config group rather than merely losing a field. - The SDK integration test "a policy rejection reaches the SDK with no structured failure fields" is preserved with a different trigger rather than deleted. It used a malformed UPN only as a vehicle; the contract it pins -- `operation`/`nativeCode`/`remediation` absent when no API call was in flight -- ships to main with #708 and applies to every policy rejection on every backend. It is also the only END-TO-END coverage of that contract; the sibling assertions in `errors.test.ts`, `state-aware.test.ts` and Rust `error.rs` all use fabricated envelopes. It now triggers on an oversized `appId` -- the same MXC-side, pre-API-call rejection. - The two `state_aware_request` secret-redaction tests are DELETED rather than rewritten. They were written specifically to demonstrate the `wamToken` path, and every link they covered is pinned elsewhere: `config_deserialize`'s own self-contained tests already assert redaction on a fully-qualified path (`experimental.someBackend.user`), and 13 non-secret tests in the same file cover prefix construction and whole-file line reporting. After this change no config field in the repo matches a secret marker, so the composition they exercised is unreachable. - The one-shot stray-config test is renamed, not dropped: it pins that an unrecognised `experimental.isolation_session` key is ignored rather than rejected, which nothing else covers. A key naming nothing real tests that better than `user` did. Docs - `docs/isolation-session/state-aware-rust.md` and `state-aware-typescript.md` are the authoritative per-backend specs: the provision/start `user` rows, the `IsolationSessionUserConfig` section, the honor-matrix rows and the Entra worked example are removed, and Start is restated as taking no per-phase config. - `docs/schema.md` loses the now-invalid `"isolation_session": { "start": { "user": … } }` nesting example. This is required by the repository convention that a config-field removal updates `docs/schema.md` alongside the generated schema (`.github/copilot-instructions.md`). The example was doubly wrong after this change: `user` no longer exists, and `start` accepts no object at all, so a caller copying it would get a hard dispatch error rather than a tolerated unknown key. - `docs/schema-codegen.md` no longer lists the `user` bundle among the objects the generated schema closes, and its per-phase nesting list is reduced to `isolation_session.provision`. (That list was also stale for `stop` / `deprovision` from #683; the whole line is corrected rather than only the part this change falsified, since a partial fix would still be wrong.) - `docs/isolation-session/oneshot.md`, `docs/windows-sandbox/windows-sandbox.md`, `docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md` and `.github/copilot-instructions.md` drop their Entra references, including three instances of the same "WindowsSandbox has no Entra `user` bundle" contrast that is meaningless once no backend has one. Deliberately not corrected here: the illustrative `"start": { "configurationId": … }` examples in `docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md`. `configurationId` was already dead before this change and the block is self-caveated as illustrative. This change does alter *why* those examples are wrong -- the shape itself is now rejected, not just the field -- but the correct content differs between this branch and the feature branch that restores the `user` bundle, so a standalone follow-up lands it once instead of being reverted and re-applied. `config_deserialize`'s secret-redaction machinery is generic infrastructure and stays; only its fixtures and the `SECRET_PATH_SEGMENTS` comment are reworded off the enterprise example. The telemetry threat-model references to UPN are not enterprise surface -- they document that the correlation-vector base is never seeded from caller identity -- and are left alone. The C# SDK is untouched: this branch does not modify `sdk/dotnet`, and it cannot reach any experimental backend today (`experimental_enabled` is never set on the mxc-sdk -> mxc_ffi path), so its dead `SandboxUserCredentials` is tracked as a separate deliverable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c82ca12-a566-4257-9de7-164be299375e

adpa-ms requested a review from a team as a code owner July 29, 2026 20:55

adpa-ms requested a review from Copilot July 30, 2026 00:35

Copilot started reviewing on behalf of adpa-ms July 30, 2026 00:36 View session

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Branden Bonaby (bbonaby) requested a review from Gudge (MGudgin) July 30, 2026 01:31

Gudge (MGudgin) requested changes Jul 31, 2026

View reviewed changes

microsoft-github-policy-service Bot assigned adpa-ms Jul 31, 2026

microsoft-github-policy-service Bot added Needs-Author-Feedback Issue needs attention from issue or PR author Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Jul 31, 2026

adpa-ms requested a review from Gudge (MGudgin) August 1, 2026 00:03

adpa-ms force-pushed the feature/isolation-session-internal branch 2 times, most recently from 29a9428 to 9dbb4c7 Compare August 4, 2026 00:32

adpa-ms and others added 3 commits August 3, 2026 17:50

adpa-ms force-pushed the user/adibpa/copilot-iso-structured-errors branch from d453262 to 478776a Compare August 4, 2026 01:22

adpa-ms merged commit 8343c88 into feature/isolation-session-internal Aug 4, 2026
17 checks passed

adpa-ms deleted the user/adibpa/copilot-iso-structured-errors branch August 4, 2026 01:33

microsoft-github-policy-service Bot removed the Needs-Attention Issue needs attention from Microsoft label Aug 4, 2026

adpa-ms mentioned this pull request Aug 6, 2026

Migrate the isolation_session backend and SDK to the IsolationSession Preview API #761

Open

5 tasks

Navigation Menu

Uh oh!

feat(iso): surface structured error fields on the state-aware wire envelope - #708

feat(iso): surface structured error fields on the state-aware wire envelope#708
adpa-ms merged 3 commits into
feature/isolation-session-internalfrom
user/adibpa/copilot-iso-structured-errors

adpa-ms commented Jul 29, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Gudge (MGudgin) left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gudge (MGudgin) Jul 31, 2026

Uh oh!

adpa-ms Aug 1, 2026

Uh oh!

Gudge (MGudgin) Jul 31, 2026

Uh oh!

adpa-ms Aug 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

adpa-ms commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

⚠️ Observable changes

🔍 Validation

✅ Checklist

📋 Issue Type

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Gudge (MGudgin) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gudge (MGudgin) Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

adpa-ms Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Gudge (MGudgin) Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

adpa-ms Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adpa-ms commented Jul 29, 2026 •

edited

Loading